fix: Stored XSS Vulnerability in Debug Toolbar (debugbar_time Paramete… #9497
Conversation
…r)!! This PR addresses a stored XSS vulnerability in the Toolbar.php file of the Debug Toolbar system. The issue arises from improper sanitization and validation of the debugbar_time GET parameter, which is used to construct file paths and read their contents. If an attacker injects malicious JavaScript into a debugbar_*.json file, it could be executed when the debug toolbar is accessed, leading to potential security risks. Issue Details: Vulnerability: The debugbar_time parameter was not properly validated, allowing attackers to inject malicious payloads. The file contents were directly echoed back to the client without escaping, enabling stored XSS attacks. Impact: Session Hijacking: Attackers could steal session cookies of admins or users. Persistent Malware Injection: Malicious scripts could persist in the debug logs and execute whenever accessed. Privilege Escalation: If executed in a privileged session, attackers could perform unauthorized actions. Fix Implemented: 1 . Input Validation: Added a preg_match() check to ensure the debugbar_time parameter only contains alphanumeric characters and underscores (^[a-zA-Z0-9_]+$). This prevents malicious input from being used to construct file paths. 2. Output Escaping: Used htmlspecialchars() to escape special characters (<, >, ", ', &) in the file contents before echoing them back to the client. This ensures that any potentially malicious content is rendered harmless in the browser. 3. File Existence and Readability Check:Added a check to ensure the file exists and is readable (is_file() and is_readable()) before attempting to read its contents. Please review and merge this PR to address the security vulnerability.
|
Hi there, shwet04! 👋 Thank you for sending this PR! We expect the following in all Pull Requests (PRs).
Important We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works. If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md Sincerely, the mergeable bot 🤖 |
shwet04
changed the title
shwet04
changed the title
|
You will be asked to delete the pr and contact by email if this is a serious problem. |
|
Hello @neznaika0 , I have shared an email -- The suggest for me was to generate a fix !! |
|
Yes, they sent the report to the appropriate channels. Since this was a debug tool that doesn't work in production, it was deemed not as high an impact as it would otherwise be. I think I do have another report from them I need to share still, though.... |
|
The PR looks like a good fix, but will need one of us to pull it down and test it out to ensure the toolbar still works. It might be a couple of days before I can get to it. |
|
I made the requested changes !! Assuming that debugbar_time considers timestamp and would allow numeric values with an optional decimal point. Can you confirm the expected format of debugbar_time (e.g., alphanumeric, timestamp, or other) ??? |
|
@shwet04 Thank you. The format is always the same: |
|
@michalsn , Can you review it now ?? Is everything okay ?? |
|
@shwet04 No, it's not - sorry. At first, I didn't pay attention to what data exactly you are trying to use with the Then I realized that we are using a Can you provide us with a working example of this vulnerability that would display, for example, an At this point, I don't think this bug report is valid. |
|
|
I was referring to the |
|
After further testing, I have not been able to generate a working XSS PoC. It appears that the application properly escapes user input before rendering. I appreciate your time reviewing this report. If I find new evidence of an issue, I will follow up with additional details. Thanks! @michalsn Can you please take time to review other findings that I have shared over Email. |
|
@shwet04, I do not know of any additional issues, but please don't share any possible vulnerabilities publicly. @lonnieezell mentioned something in one of his previous posts here, but I believe he will have to evaluate it first If that's it for now, feel free to close this PR. |
|
@shwet04 We have carefully analyzed the findings sent to us, and none of them represent a real vulnerability.
Anyway, thank you for your report and for trying to make CodeIgniter more secure. |
4 participants
This PR addresses a stored XSS vulnerability in the Toolbar.php file of the Debug Toolbar system. The issue arises from improper sanitization and validation of the debugbar_time GET parameter, which is used to construct file paths and read their contents. If an attacker injects malicious JavaScript into a debugbar_*.json file, it could be executed when the debug toolbar is accessed, leading to potential security risks.
Issue Details:
Vulnerability:
The debugbar_time parameter was not properly validated, allowing attackers to inject malicious payloads. The file contents were directly echoed back to the client without escaping, enabling stored XSS attacks.
Impact:
Fix Implemented:
1 . Input Validation: Added a preg_match() check to ensure the debugbar_time parameter only contains alphanumeric characters and underscores (^[a-zA-Z0-9_]+$). This prevents malicious input from being used to construct file paths.
Output Escaping: Used htmlspecialchars() to escape special characters (<, >, ", ', &) in the file contents before echoing them back to the client. This ensures that any potentially malicious content is rendered harmless in the browser.
File Existence and Readability Check:Added a check to ensure the file exists and is readable (is_file() and is_readable()) before attempting to read its contents.
Please review and merge this PR to address the security vulnerability.